Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[chore] build compatible distributions as PIE #726

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

jackgopack4
Copy link
Contributor

@jackgopack4 jackgopack4 commented Nov 5, 2024

Currently, the collector distributions and OCB are not built as position-independent executables.
According to the OTel Collector Security Audit, this could cause potential vulnerabilities: #618

This PR updates the goreleaser template and the corresponding YAML files to run two separate builds for each distribution that come together for one release each; if the platform and architecture support Internal Linking in Golang, they are built with flag -buildmode=pie. If not, they are built the same way as before.

You can see the sample releases in my forked repository:
https://github.com/jackgopack4/opentelemetry-collector-releases/releases/tag/v0.114.0
https://github.com/jackgopack4/opentelemetry-collector-releases/releases/tag/cmd%2Fbuilder%2Fv0.114.0
https://hub.docker.com/r/johnpeterson785/opentelemetry-collector/tags
https://hub.docker.com/r/johnpeterson785/opentelemetry-collector-contrib/tags
https://hub.docker.com/r/johnpeterson785/opentelemetry-collector-otlp/tags
https://hub.docker.com/r/johnpeterson785/opentelemetry-collector-k8s/tags

I took the approach of generating an "ignore" list for PIE vs not-PIE and otherwise leaving everything else the same.

@jackgopack4 jackgopack4 marked this pull request as ready for review November 5, 2024 20:03
@jackgopack4 jackgopack4 requested review from a team as code owners November 5, 2024 20:03
jackgopack4 added a commit to jackgopack4/opentelemetry-collector-releases that referenced this pull request Nov 6, 2024
Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR has changes from #708. I'd prefer not to mix the concepts. I don't care which one merges first but if you want it do be this one then we need to remove the changes that allow updating the k8s and otlp distros with internal library

@jackgopack4
Copy link
Contributor Author

my bad, I think I branched this one off of that one; I will make sure to remove those changes from here

@jackgopack4
Copy link
Contributor Author

ok I've completely separated the two. I don't mind which one merges first, whichever one I'll be able to rebase/merge the remaining branch

TylerHelmuth added a commit that referenced this pull request Nov 7, 2024
* build ocb as position-independent executable

* add -buildmode=pie flags for go binary builds

* add generation for otlp and k8s goreleaser

* remove faulty buildmode pie logic

* remove buildmode==pie in favor of #726

* add constants for distro names

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Tyler Helmuth <[email protected]>
@jackgopack4 jackgopack4 marked this pull request as draft November 7, 2024 22:10
@jackgopack4
Copy link
Contributor Author

marked as draft until I can resolve merge conflicts after #708 merge

@TylerHelmuth
Copy link
Member

@jackgopack4 this is ready to pick up again since #708 is merged

@jackgopack4
Copy link
Contributor Author

Thanks, yes I hadn't gotten a chance but I'll take another crack at it this week

@jackgopack4
Copy link
Contributor Author

PR is ready for review, the release artifacts in my fork match exactly the release artifacts in the open-telemetry repo.
https://github.com/jackgopack4/opentelemetry-collector-releases/releases/tag/v0.114.0
https://github.com/jackgopack4/opentelemetry-collector-releases/releases/tag/cmd%2Fbuilder%2Fv0.114.0

@jackgopack4 jackgopack4 marked this pull request as ready for review November 20, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants